Skip to content

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

cuviper and others added 14 commits November 19, 2021 19:33
`Module::getOrInsertGlobal` returns a `Constant*`, which is a super
class of `GlobalVariable`, but if the given type doesn't match an
existing declaration, it returns a bitcast of that global instead.
This causes UB when we pass that to `LLVMGetVisibility` which
unconditionally casts the opaque argument to a `GlobalValue*`.

Instead, we can do our own get-or-insert without worrying whether
existing types match exactly. It's not relevant when we're just trying
to get/set the linkage and visibility, and if types are needed we can
bitcast or error nicely from `rustc_codegen_llvm` instead.
…tmcm

Adds IEEE 754-2019 minimun and maximum functions for f32/f64

IEEE 754-2019 removed the `minNum` (`min` in Rust) and `maxNum` (`max` in Rust) operations in favor of the newly created `minimum` and `maximum` operations due to their [non-associativity](https://grouper.ieee.org/groups/msc/ANSI_IEEE-Std-754-2019/background/minNum_maxNum_Removal_Demotion_v3.pdf) that cannot be fix in a backwards compatible manner. This PR adds `fN::{minimun,maximum}` functions following the new rules.

### IEEE 754-2019 Rules

> **minimum(x, y)** is x if x < y, y if y < x, and a quiet NaN if either operand is a NaN, according to 6.2.
For this operation, −0 compares less than +0. Otherwise (i.e., when x = y and signs are the same)
it is either x or y.

> **maximum(x, y)** is x if x > y, y if y > x, and a quiet NaN if either operand is a NaN, according to 6.2.
For this operation, +0 compares greater than −0. Otherwise (i.e., when x = y and signs are the
same) it is either x or y.

"IEEE Standard for Floating-Point Arithmetic," in IEEE Std 754-2019 (Revision of IEEE 754-2008) , vol., no., pp.1-84, 22 July 2019, doi: 10.1109/IEEESTD.2019.8766229.

### Implementation

This implementation is inspired by the one in [`glibc` ](https://github.com/bminor/glibc/blob/90f0ac10a74b2d43b5a65aab4be40565e359be43/math/s_fminimum_template.c) (it self derived from the C2X draft) expect that:
 - it doesn't use `copysign` because it's not available in `core` and also because `copysign` is unnecessary (we only want to check the sign, no need to create a new float)
 - it also prefer `other > self` instead of `self < other` like IEEE 754-2019 does

I originally tried to implement them [using intrinsics](Urgau@1d8aa13) but LLVM [error out](https://godbolt.org/z/7sMrxW49a) when trying to lower them to machine intructions, GCC doesn't yet have built-ins for them, only cranelift support them nativelly (as it doesn't support the nativelly the old sementics).

Helps with rust-lang#83984
Make `LLVMRustGetOrInsertGlobal` always return a `GlobalVariable`

`Module::getOrInsertGlobal` returns a `Constant*`, which is a super
class of `GlobalVariable`, but if the given type doesn't match an
existing declaration, it returns a bitcast of that global instead.
This causes UB when we pass that to `LLVMGetVisibility` which
unconditionally casts the opaque argument to a `GlobalValue*`.

Instead, we can do our own get-or-insert without worrying whether
existing types match exactly. It's not relevant when we're just trying
to get/set the linkage and visibility, and if types are needed we can
bitcast or error nicely from `rustc_codegen_llvm` instead.

Fixes rust-lang#91050, fixes rust-lang#87933, fixes rust-lang#87813.
…t, r=estebank

Add spaces in opaque `impl Trait` with more than one trait

`impl A+B` becomes `impl A + B`

r? `@estebank`
…bank

Don't suggest certain fixups (`.field`, `.await`, etc) when reporting errors while matching on arrays

When we have a type mismatch with a `cause.code` that is an `ObligationCauseCode::Pattern`, skip suggesting fixes like adding `.await` or accessing a struct's `.field` if the pattern's `root_ty` differs from the `expected` ty. This occurs in situations like this:

```rust
struct S(());

fn main() {
    let array = [S(())];

    match array {
        [()] => {}
        _ => {}
    }
}
```

I think what's happening here is a layer of `[_; N]` is peeled off of both types and we end up seeing the mismatch between just `S` and `()`, but when we suggest a fixup, that applies to the expression with type `root_ty`.

---

Questions:
1. Should this check live here, above all of the suggestions, or should I push this down into every suggestion when we match `ObligationCauseCode`?
2. Any other `ObligationCauseCode`s to check here?
3. Am I overlooking an easier way to get to this same conclusion without pattern matching on `ObligationCauseCode` and comparing `root_ty`?

Fixes rust-lang#91058
@rustbot rustbot added the rollup A PR which is a rollup label Nov 21, 2021
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=4

@bors
Copy link
Collaborator

bors commented Nov 21, 2021

📌 Commit a54eae9 has been approved by matthiaskrgr

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 21, 2021
@bors
Copy link
Collaborator

bors commented Nov 21, 2021

⌛ Testing commit a54eae9 with merge 3bfde2f...

@bors
Copy link
Collaborator

bors commented Nov 21, 2021

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 3bfde2f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 21, 2021
@bors bors merged commit 3bfde2f into rust-lang:master Nov 21, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 21, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3bfde2f): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@matthiaskrgr matthiaskrgr deleted the rollup-duk33o1 branch November 25, 2021 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants